Fixes #2655 Usage of SimulationBuilder in R#2698
Conversation
| { | ||
| var cache = new CacheByName<MoleculeBuilder>(); | ||
| allMoleculeBuilders.Where(x => moleculeNames.Contains(x.Name)).Each(x => cache.Add(x)); | ||
| return cache.ToList(); |
There was a problem hiding this comment.
Cache by name so that we only pay attention to the last merged version of a builder.
| CoreSimulation = modelCoreSimulation; | ||
|
|
||
| // Initialize the list of molecule builders for which there are initial conditions in the simulation | ||
| _allBuildersInSimulation = allBuildersFor(allMoleculeNamesInSimulation.ToList()).DistinctBy(x => x.Name).ToList(); |
There was a problem hiding this comment.
I think we can build this list once at construction rather than build the list for each access.
|
I'll add some tests here once I get some feedback that I'm on the right track. I think this is pretty much how the model constructor does adds or ignores molecules when they don't have initial conditions. |
msevestre
left a comment
msevestre
left a comment
There was a problem hiding this comment.
No changes are required on the .NET side anymore THe service that I have created needs to be instantiated in R and used in the simulatuon object
| Where(x => x.IsPresent). // this initial condition is present | ||
| Select(ic => ic.MoleculeName).Distinct(); | ||
|
|
||
| private IEnumerable<MoleculeBuilder> allMoleculeBuilders => |
There was a problem hiding this comment.
none of this code is required . The service returning a simulation builder should be used IMO in the simulation.R alongside the simulation and saved as reference to be used with each subsequent access
|
So this should be closed and the issue closed as well? |
|
Once the R counterpart is implemented then probably. |
2 participants
Fixes #2655
Description
expose molecules by property after removing
SimulationBuilderType of change
Please mark relevant options with an
xin the brackets.How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Reviewer checklist
Mark everything that needs to be checked before merging the PR.
This change requires a documentation updateabove is selectedScreenshots (if appropriate):
Questions (if appropriate):